Skip to content

Add fmt_print and fmt_println to C API#4789

Open
Ferdi265 wants to merge 2 commits into
fmtlib:mainfrom
Ferdi265:feature/fmt-c-print
Open

Add fmt_print and fmt_println to C API#4789
Ferdi265 wants to merge 2 commits into
fmtlib:mainfrom
Ferdi265:feature/fmt-c-print

Conversation

@Ferdi265
Copy link
Copy Markdown
Contributor

This PR introduces fmt_print() and fmt_println() macros to the C API, which print to a FILE* stream using fmt::vprint(stream, fmt, format_args).

Additionally, this PR introduces the following helpers in fmt-c.h:

  • a helper macro FMT_FORMAT_ARGS(fmt, ...) that abstracts away the macro magic necessary to produce the fmt_arg array, useful for avoiding duplicated logic when declaring additional wrapper macros that expand to calls to fmt_vformat or similar
  • a fmt_vprint() function used to implement the fmt_print() macro
  • a fmt_vprintln() function used to implement the fmt_println() macro

and the following helpers in fmt-c.cc:

  • a helper template fmt_c_wrapper() that performs the conversion from fmt_arg to fmt::basic_format_arg, calls the passed lambda, and handles exceptions, to reduce boilerplate and duplication between fmt_vformat(), fmt_vprint(), and fmt_vprintln()

Copy link
Copy Markdown
Contributor

@vitaut vitaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. I think adding fmt_print makes sense as a safe replacement for printf but I don't think we need fmt_println - let's keep the API small for now.

Comment thread src/fmt-c.cc
constexpr size_t max_args = 16;
if (num_args > max_args) return fmt_error_invalid_arg;
constexpr size_t max_c_format_args = 16;
static int parse_c_format_args(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not parsing, maybe rename to convert?

Comment thread src/fmt-c.cc
Comment on lines +39 to +40
static int fmt_c_wrapper(const char* fmt, const fmt_arg* args, size_t num_args,
F&& f) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants